-
Notifications
You must be signed in to change notification settings - Fork 17.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Move RATE logging to attitude controller and log sample time #27996
Conversation
70e5ed1
to
76669b4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great change but I would like to do the same thing with the ATT message.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be a nice clean up.
76669b4
to
ed9f731
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this looks good. The only question is if any log tools have a problem with the log timestamp not matching the time the log was written. I don't think they should. Each message should still see a increasing time. Only between messages would time go backwards. The only other thing would be if we were to write the log twice in one loop for some reason then we would get two timestamps the same.
I guess we could get round this by adding a new timestamp rather than replacing the existing one, but that would make plotting in log review tools hard as there mostly hard coded to use TimeUS
on the x axis.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks
As discussed on the call, a few of us are worried about the TimeUS potentially going backwards which could confuse some log analysis tools but I think it's worth the risk and it seems important that we give the devs the data they need
move RATE log structure to AC_AttitudeControl
move Write_Rate() to AC_AttitudeControl move RATE log structure to AC_AttitudeControl
ed9f731
to
5b85673
Compare
With RATE logging it is crucial that both the last gyro value used and the time that it was used be recorded in the log rather than whatever happens to be available at the time the log is written. In addition it makes no sense for RATE logging to be in AHRS, it really has very little to do with AHRS and everything to do with the rate and attitude controllers. This PR moves the logging functionality and records the gyro value used and timestamp at the appropriate point for logging. This is especially important when the rate loop because faster and concurrent.
Split from #27029